Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cxx-qt-lib: build a QSet<T> #350

Merged
merged 4 commits into from
Dec 1, 2022
Merged

cxx-qt-lib: build a QSet<T> #350

merged 4 commits into from
Dec 1, 2022

Conversation

ahayzen-kdab
Copy link
Collaborator

@ahayzen-kdab ahayzen-kdab commented Nov 4, 2022 •
edited
Loading

Related to #347

@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 5 times, most recently from d15d606 to 524b8f0 Compare November 4, 2022 13:46
@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 2 times, most recently from de1c97c to 5e39b07 Compare November 4, 2022 16:08
@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 2 times, most recently from 6ef45a4 to 62b75e0 Compare November 10, 2022 11:46
@ahayzen-kdab ahayzen-kdab mentioned this pull request Nov 10, 2022
Closed
6 tasks
@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 4 times, most recently from 4166d2a to 746459f Compare November 10, 2022 18:55
@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 3 times, most recently from 7ed0863 to e9cf270 Compare November 11, 2022 10:02
@ahayzen-kdab ahayzen-kdab changed the title WIP: cxx-qt-lib: build a QSet<T> cxx-qt-lib: build a QSet<T> Nov 11, 2022
@ahayzen-kdab ahayzen-kdab marked this pull request as ready for review November 11, 2022 10:03
@ahayzen-kdab ahayzen-kdab mentioned this pull request Nov 11, 2022
Merged
@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 2 times, most recently from 944b429 to e94e7e7 Compare November 11, 2022 17:03
@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 3 times, most recently from ecee3a3 to 21ebcfe Compare November 16, 2022 14:27
LeonMatthesKDAB
LeonMatthesKDAB requested changes Nov 23, 2022
View reviewed changes
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could simplify this using CXX instead of C-style FFI.

crates/cxx-qt-lib-headers/include/qset.h Show resolved Hide resolved
crates/cxx-qt-lib/build.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/types/qset.cpp Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/types/qset/generate.sh Show resolved Hide resolved
@LeonMatthesKDAB
Copy link
Collaborator

Initial version of the templated idea:
0002-Use-template-methods-for-clone-etc.patch.txt

@ahayzen-kdab
Copy link
Collaborator Author

Initial version of the templated idea: 0002-Use-template-methods-for-clone-etc.patch.txt

Done

@ahayzen-kdab ahayzen-kdab force-pushed the 347-container-types branch 3 times, most recently from 64ac386 to 1f58fcc Compare November 23, 2022 18:30
@Be-ing
Copy link
Contributor

Be-ing commented Nov 23, 2022

It would help to incorporate this into one of the examples to see how the API works in practice.

LeonMatthesKDAB
LeonMatthesKDAB requested changes Nov 24, 2022
View reviewed changes
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall code is good, I just have a few more nitpicks, then ready for merge IMHO.

crates/cxx-qt-lib/build.rs Outdated Show resolved Hide resolved
crates/cxx-qt-lib/src/types/qset/generate.sh Show resolved Hide resolved
crates/cxx-qt-lib/src/types/qset.cpp Outdated Show resolved Hide resolved
crates/cxx-qt-lib-headers/include/qset.h Show resolved Hide resolved
crates/cxx-qt-lib/src/types/qset.cpp Outdated Show resolved Hide resolved
tests/qt_types_standalone/rust/src/qset.rs Outdated Show resolved Hide resolved
@ahayzen-kdab
Copy link
Collaborator Author

It would help to incorporate this into one of the examples to see how the API works in practice.

Added a containers section to the qml_features so we can have QSet/QHash/QVector etc all here later.

Also note that you can see how these are used in qt_types_standalone as well.

ahayzen-kdab and others added 4 commits December 1, 2022 12:51
@ahayzen-kdab
cxx-qt-lib: define a QSet<T> with QSetElement trait
b864725 
Also define all primitive types and cxx_qt_lib types as T.

Related to KDAB#347
@LeonMatthesKDAB @ahayzen-kdab
cxx-qt-lib: use CXX-style FFI for methods which can be defined
63e24c7 
Some methods can be defined via the CXX bridge so don't need a
C++ proxy between.
@LeonMatthesKDAB @ahayzen-kdab
cxx-qt-lib: Use template methods for QSet methods that can't be defined
e8eb3ee 
Methods that can't be defined using CXX use template methods.
@ahayzen-kdab
examples: add containers page for QSet and later hash, vector
ea2c333
LeonMatthesKDAB
LeonMatthesKDAB approved these changes Dec 1, 2022
View reviewed changes
Copy link
Collaborator

@LeonMatthesKDAB LeonMatthesKDAB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ahayzen-kdab ahayzen-kdab merged commit f844a8e into KDAB:main Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Be-ing Be-ing Be-ing left review comments

@LeonMatthesKDAB LeonMatthesKDAB LeonMatthesKDAB approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ahayzen-kdab @LeonMatthesKDAB @Be-ing